Pro 7798 fix ordering - #4994
Conversation
boutell
left a comment
boutell
left a comment
There was a problem hiding this comment.
This feels right. Unfortunately I don't have time to test it thoroughly but the mocha test coverage looks solid.
myovchev
left a comment
myovchev
left a comment
There was a problem hiding this comment.
A tiny code style thing, it looks good.
| } | ||
| }); | ||
| }); | ||
| } else { |
There was a problem hiding this comment.
You don't need else here. Just return at the end of the previous if block(becauseelse` ends the function). It's more readable.
There was a problem hiding this comment.
You are missing ### Adds in the CHANGELOG. Also, as discussed with @haroun, we would like to create all sub-titles, even empty (Adds, Changes, etc) initially to reduce the merge conflicts. It's a good timing for doing it.
@BoDonkey You probably missed this one, I don't see any change in the changelog related to that. |
3 participants
Summary
Summarize the changes briefly, including which issue/ticket this resolves. If it closes an existing Github issue, include "Closes #[issue number]"
This PR implements better menu arrangement options for the admin-bar module. It focuses on groups first, but with order as a backup for organizing non-grouped items. It also increases the tests to cover functionality better. Closes PRO-7798.
What are the specific steps to test this change?
For example:
What kind of change does this PR introduce?
(Check at least one)
Make sure the PR fulfills these requirements:
If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.
Other information: